src/CompositeKey.SourceGeneration.FunctionalTests/PrimaryKeys.cs (1)

1-1: Missing functional test for ImmutableArray<T>.

The using System.Collections.Immutable import is added, but no record in this file actually exercises ImmutableArray<T> as a repeating property type. Since the PR objectives explicitly list ImmutableArray<T> as a supported collection type alongside List<T> and IReadOnlyList<T>, consider adding a test record such as:

[CompositeKey("{Ids...#}")]
public sealed partial record ImmutableArrayRepeatingKey(ImmutableArray<Guid> Ids);

This would ensure end-to-end coverage for all three supported collection types.

src/CompositeKey.SourceGeneration.UnitTests/SourceGeneratorTests.cs (3)

549-573: Consider aligning the success-test pattern with existing tests.

Other success tests (e.g. BasicPrimaryKey_ShouldSuccessfullyGenerateSource at line 30) call CompilationHelper.RunSourceGenerator(compilation) without disableDiagnosticValidation: true, relying on the helper's built-in validation. Here, you disable that validation and manually filter to > DiagnosticSeverity.Info.

If these compilations are expected to succeed cleanly, dropping disableDiagnosticValidation: true would give you the same strict validation as every other happy-path test. If there's a reason Info-level diagnostics are emitted that would trip the default validation, it would be worth a brief comment explaining why.

---

575-617: Failure-test assertions are weaker than the established pattern.

The existing failure tests (e.g. NonRecordType_ShouldFailCompilation) assert severity, location, and message via CompilationHelper.AssertDiagnostics. These two new tests only check that a diagnostic with the expected ID exists. This means a regression that changes the severity to Warning or produces a garbled message would still pass.

Consider matching the stricter pattern used elsewhere — resolve the location and assert the full DiagnosticData — so these diagnostics are validated as thoroughly as the rest.

---

595-617: No test coverage for COMPOSITE0011 ("repeating property must be the last part").

COMPOSITE0009 and COMPOSITE0010 each have a dedicated test, but COMPOSITE0011 appears to be missing. Adding a test with a repeating section followed by another part (e.g. [CompositeKey("TAG_{Tags...#}_{Suffix}")]) would close this gap.

Would you like me to draft the test case for COMPOSITE0011, or open an issue to track it?

src/CompositeKey.SourceGeneration.FunctionalTests/CompositePrimaryKeyTests.cs (4)

500-507: Consider adding edge-case entries to the invalid primary keys data.

A few notable cases are missing:

1. An empty repeating section, e.g. "eccd98c4-d484-4429-896d-8fcdd77c6327|LOCATION#" — per the PR description, empty collections should throw FormatException, so this is a valuable boundary test.
2. A key with the correct constant prefix but no # separator after it, e.g. "eccd98c4-d484-4429-896d-8fcdd77c6327|LOCATION".

These would strengthen confidence in the parser's handling of malformed repeating sections.

Suggested additions

 public static object[][] CompositeWithRepeatingSort_InvalidPrimaryKeys() =>
 [
     [""],
     ["a"],
     ["a|b"],
     ["not-a-guid|LOCATION#not-a-guid"],
-    ["eccd98c4-d484-4429-896d-8fcdd77c6327|WRONG#eccd98c4-d484-4429-896d-8fcdd77c6328"]
+    ["eccd98c4-d484-4429-896d-8fcdd77c6327|WRONG#eccd98c4-d484-4429-896d-8fcdd77c6328"],
+    ["eccd98c4-d484-4429-896d-8fcdd77c6327|LOCATION#"],
+    ["eccd98c4-d484-4429-896d-8fcdd77c6327|LOCATION"]
 ];

---

509-515: Similarly, the partition+sort key invalid data could cover an empty repeating section.

Adding a case with an empty sort-key repeating part (e.g. "LOCATION#") would mirror the suggestion above.

Suggested addition

 public static object[][] CompositeWithRepeatingSort_InvalidPartitionKeyAndSortKeys() =>
 [
     ["a", "b"],
     ["not-a-guid", "LOCATION#not-a-guid"],
     ["eccd98c4-d484-4429-896d-8fcdd77c6327", "WRONG#eccd98c4-d484-4429-896d-8fcdd77c6328"],
-    ["eccd98c4-d484-4429-896d-8fcdd77c6327", "LOCATION#not-a-guid"]
+    ["eccd98c4-d484-4429-896d-8fcdd77c6327", "LOCATION#not-a-guid"],
+    ["eccd98c4-d484-4429-896d-8fcdd77c6327", "LOCATION#"],
+    ["eccd98c4-d484-4429-896d-8fcdd77c6327", "LOCATION"]
 ];

---

326-404: Missing tests for empty-collection runtime behaviour and invalid ToSortKeyString part indices.

Two gaps compared to the existing CompositePrimaryKey test patterns:

1. Empty collection at runtime — the PR states empty collections throw FormatException, but there is no test exercising ToString() / ToSortKeyString() on a CompositeWithRepeatingSort constructed with an empty LocationId list.

2. Invalid ToSortKeyString part index — the existing CompositePrimaryKey region includes explicit ShouldThrowInvalidOperationException tests for out-of-range throughPartIndex values (lines 123–131). The new region lacks an equivalent test for CompositeWithRepeatingSort.ToSortKeyString with an excessive index.

---

377-404: Partial-key test only exercises a 3-element collection — consider a single-element case too.

The test constructs ids with three GUIDs, so the repeating section always has multiple elements. A single-element list would verify the boundary where there is exactly one repeated part and no inter-element separator is emitted, which is a common off-by-one risk area.

src/CompositeKey.Analyzers.Common/Validation/TemplateValidation.cs (1)

89-112: Consider extracting shared property-accessibility validation to reduce duplication.

Lines 64–87 and 89–112 perform identical accessibility checks — one for PropertyTemplateToken and one for RepeatingPropertyTemplateToken. A shared helper that accepts token name, getter, and setter flags would eliminate this duplication.

♻️ Sketch: extract a common helper

+    private static TemplateValidationResult ValidateTokenPropertyAccessibility(
+        string tokenName,
+        List<PropertyInfo> availableProperties)
+    {
+        var property = availableProperties.FirstOrDefault(p => p.Name == tokenName);
+        if (property == null)
+        {
+            return TemplateValidationResult.Failure(
+                DiagnosticDescriptors.PropertyMustHaveAccessibleGetterAndSetter,
+                tokenName);
+        }
+
+        var accessibilityInfo = new PropertyValidation.PropertyAccessibilityInfo(
+            property.Name, property.HasGetter, property.HasSetter);
+
+        var result = PropertyValidation.ValidatePropertyAccessibility(accessibilityInfo);
+        if (!result.IsSuccess)
+        {
+            return TemplateValidationResult.Failure(result.Descriptor, result.MessageArgs);
+        }
+
+        return TemplateValidationResult.Success();
+    }

Then both loops can call ValidateTokenPropertyAccessibility(token.Name, availableProperties).

src/CompositeKey.Analyzers/Analyzers/PropertyAnalyzer.cs (2)

275-302: Consider extracting shared type-info creation logic.

CreateInnerTypeInfo duplicates most of the logic from CreatePropertyTypeInfo (Lines 307–329), differing only in the source ITypeSymbol. Both could delegate to a shared CreateTypeInfo(ITypeSymbol type, Compilation compilation) helper.

♻️ Suggested shared helper

+    private static PropertyValidation.PropertyTypeInfo CreateTypeInfoFromSymbol(
+        ITypeSymbol type,
+        Compilation compilation)
+    {
+        var guidType = compilation.GetTypeByMetadataName("System.Guid");
+        var stringType = compilation.GetSpecialType(SpecialType.System_String);
+
+        var isGuid = SymbolEqualityComparer.Default.Equals(type, guidType);
+        var isString = SymbolEqualityComparer.Default.Equals(type, stringType);
+        var isEnum = type.TypeKind == TypeKind.Enum;
+
+        var interfaces = type.AllInterfaces;
+        var isSpanParsable = interfaces.Any(i => i.ToDisplayString().StartsWith("System.ISpanParsable", StringComparison.Ordinal));
+        var isSpanFormattable = interfaces.Any(i => i.ToDisplayString().Equals("System.ISpanFormattable", StringComparison.Ordinal));
+
+        return new PropertyValidation.PropertyTypeInfo(
+            TypeName: type.ToDisplayString(),
+            IsGuid: isGuid,
+            IsString: isString,
+            IsEnum: isEnum,
+            IsSpanParsable: isSpanParsable,
+            IsSpanFormattable: isSpanFormattable);
+    }

Then both CreatePropertyTypeInfo and CreateInnerTypeInfo can delegate to it.

---

59-126: Accessibility validation is duplicated across both branches.

The PropertyAccessibilityInfo construction and ValidatePropertyAccessibility call at Lines 65–78 is repeated nearly identically in the RepeatingPropertyTemplateToken branch at Lines 133–146. Consider extracting into a small helper to reduce duplication, though this is a minor nit given the current scope.

src/CompositeKey.SourceGeneration/SourceGenerator.Emitter.cs (2)

892-916: Fixed-part switch cases lack a catch-all for throughPartIndex values that fall into the repeating range.

WriteFixedPartCases emits a switch statement with explicit cases only for fixed parts. If throughPartIndex equals or exceeds fixedPartCount, execution falls through the switch without matching and continues to WriteRepeatingPartHandler, which is the intended flow. This works because the switch has no default arm — control simply falls past the block.

This is correct but could be fragile if someone later adds a default: throw arm. A brief inline comment in the generated code (or a code comment here) noting the intentional fall-through would improve maintainability.

---

935-946: Minor: Intermediate string allocation for fixed prefix in repeating dynamic format.

Lines 943–945 generate code like handler.AppendFormatted(string.Create(..., $"PREFIX#{Prop}")), which creates a temporary string for the fixed prefix before appending to the handler. For the dynamic format method this is called infrequently (once per invocation), so the impact is negligible, but it's worth noting for future optimisation if needed.

src/CompositeKey.SourceGeneration/SourceGenerator.Parser.cs (1)

185-193: Redundant validation — already enforced in ToPropertyKeyPart.

The check at Lines 188–192 verifies that no PropertyKeyPart has CollectionType != None. However, ToPropertyKeyPart (Line 251) already rejects collection-type properties and returns null, which causes ParseTemplateStringIntoKeyParts to bail out before reaching this block. Any PropertyKeyPart that made it into the keyParts list is guaranteed to have CollectionType == None.

This block is dead code and can be removed to avoid confusion.

♻️ Proposed removal

-            // Validate: repeating type used without repeating syntax -> COMPOSITE0010
-            foreach (var keyPart in keyParts)
-            {
-                if (keyPart is PropertyKeyPart pkp && pkp.Property.CollectionType != CollectionType.None)
-                {
-                    ReportDiagnostic(DiagnosticDescriptors.RepeatingTypeMustUseRepeatingSyntax, _location, pkp.Property.Name);
-                    return null;
-                }
-            }

src/CompositeKey.Analyzers.Common.UnitTests/Validation/TemplateValidationTests.cs (1)

789-894: Consider adding a test for a repeating property as the last part in the partition key.

The position tests cover the sort-key side (CompositeKey_RepeatingPropertyLastInSortKey, CompositeKey_RepeatingPropertyNotLastInSortKey) but there's no test for a repeating property being (correctly) last in the partition key section of a composite key, e.g.:

{UserId}#{Tags...,} # {PostId}
 ^sort                  ^partition has repeating last

This would exercise the partition-specific validation path (Lines 213–219 in the Parser) and help catch issues like the global-vs-per-section validation bug noted in the Parser review.